Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lock Gemfile-shopify to money 0.14.x #353

Merged
merged 1 commit into from
Jan 20, 2021
Merged

Lock Gemfile-shopify to money 0.14.x #353

merged 1 commit into from
Jan 20, 2021

Conversation

zbriscoe
Copy link
Contributor

@zbriscoe zbriscoe commented Jan 20, 2021

The Gemfile.shopify file doesn't specify a version of money to use. Not having that can lead to issues.

A particular problem currently caused by this (and what is prompting me to specify it) is that Money 0.15 was just released which is causing several tests to fail.

Let's keep the minor version locked for now to prevent breakages introduced by functionality changes.

We can bump the money gem to 0.15 in a follow up PR, for now we need to create a release for a fix for Gestpay and get the master CI green.

The `Gemfile.shopify` file doesn't specify a version of money to use.
I'm not entirely sure _why_ that's the case, but it seems like an
unnecessary risk to me that can just lead to headaches.

A particular headache I'm currently experiencing (and that's
prompting me to lock this) is that Money 0.15 was just released
and several tests began to fail because of that.

Let's keep the minor version locked for now to prevent breakages
introduced by functionality changes.
@zbriscoe zbriscoe requested a review from bdewater January 20, 2021 20:24
Copy link
Contributor

@bdewater bdewater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can bump the money gem to 0.15 in a follow up PR

Please do, I have concerns that locking this version without a follow-up fix will cause breakage in applications that use both offsite-payments and money.

On a side note, I wish I had merged #248 😅

@zbriscoe zbriscoe merged commit 7faacfd into master Jan 20, 2021
@pi3r
Copy link
Contributor

pi3r commented Jan 20, 2021

Please do, I have concerns that locking this version without a follow-up fix will cause breakage in applications that use both offsite-payments and money.

Oh don't you worry, only core uses both, no biggie.

@pi3r pi3r deleted the lock-money-version branch January 20, 2021 20:40
@zbriscoe
Copy link
Contributor Author

@bdewater Do you have someone who can look at this immediately? Without setting the version the CI will continue to fail on master. And I don't have the bandwidth or context for this right now unfortunately.

If there are serious concerns and there is someone available shortly to do this I can wait to release for a bit until we can bump to 0.15. Question though, if there were any real problems in core we'd be experiencing them now since this gem (already) doesn't play nice with 0.15, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants